-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(DataTable): fix cell size and border in FF #2072
Conversation
- add classes for (header) cell containers - fix cell height size - tune up tests, stories, and snapshots
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2072 +/- ##
=======================================
Coverage 97.08% 97.08%
=======================================
Files 111 111
Lines 2740 2740
Branches 726 726
=======================================
Hits 2660 2660
Misses 77 77
Partials 3 3 ☔ View full report in Codecov by Sentry. |
size-limit report 📦
|
There is one remaining issue noticed here, with the right-hand border in the right-most header cell. Will address in a separate PR, with any other firefox discrepancies |
@@ -213,6 +213,7 @@ export function DataTable<T>({ | |||
<DataTableRow key={headerGroup.id}> | |||
{headerGroup.headers.map((header) => ( | |||
<th | |||
className={styles['data-table__header-cell-container']} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a complaint, but curious about this naming format that mizes underscore and dash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep yep, doc.s here
In our scheme, -
has no special meaning and is in place of using camelCase in CSS naming, preferring snake-case. --
and __
have more strict meaning, with most of the rest is convention
HOLD ME ACCOUNTABLE to that page :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
thanks @sskikne ✅ |
## [15.5.0](v15.4.1...v15.5.0) (2024-10-18) [Storybook](https://61313967cde49b003ae2a860-gtyxxzokxd.chromatic.com/) ### Features * **DataTable:** add support for table row statuses ([#2073](#2073)) ([c109f52](c109f52)) * re-export HeadlessUI's Transition component ([#2069](#2069)) ([a4a5fc8](a4a5fc8)) ### Bug Fixes * **DataTable:** fix cell size and border in FF ([#2072](#2072)) ([7ffbd56](7ffbd56)) * **Select:** remove label checks from component ([#2068](#2068)) ([8ede09d](8ede09d))
Test Plan: